-
Notifications
You must be signed in to change notification settings - Fork 2.8k
fix: OPTIC-945: Improve the bundling strategy and ship a single entrypoint with a common vendor chunk across all libs #6152
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…ze shared common libs
…referenced from the ls app
/docker build
|
/docker build
|
/docker build
|
/git merge develop
|
web/apps/labelstudio/src/pages/CreateProject/Config/Preview.jsx
Outdated
Show resolved
Hide resolved
/docker build
|
PR fulfills these requirements
[fix|feat|ci|chore|doc]: TICKET-ID: Short description of change made
ex.fix: DEV-XXXX: Removed inconsistent code usage causing intermittent errors
Change has impacts in these area(s)
(check all that apply)
Describe the reason for change
This change aligns the cross library component usage with React, such that it can and is now bundling only 1 set of dependencies across all of the code shipped as part of the frontend. The primary driving force of this change is to enable better code bundling strategies, reduce the amount of code shipped, fix our React usage which had become tied to workarounds from the triplicated definitions of React in various bundles and align the codebase to being able to start extracting common components and utils to a shared space. This PR now allows us to adjust, measure and update the bundling strategy further than proposed here, and will also allow a component by component reduction in the codebase. The end goal being, to ship less code to the end user more granularly with improved performance and consistency.
Functionally and stylistically this should remain exactly as previous. The only difference is in how we are bundling the code, and with which parts are put in specific bundles to retain a better longer term caching strategy between releases.
To achieve this single bundle we had to opt to de-duplicate the components which conflicted in css definitions, which ultimately based on the findings of the current state was more tenable to hardcode the prefix of a given component in conflict as a postfix of the block. This is backwards compatible and also addresses the issue in unifying our build.
Notes to reviewers
Explicitly this change has a direct dependency on the decommissioning and removal of the FF
fflag_fix_front_lsdv_4620_memory_leaks_100723_short
. The reason is, we are now only using a single React and this change severs React fibers which are still in use by the remainder of page and does not allow React to properly flush pendingContext as a result, which causes errors and functionality to break. As part of this change we are going to be rolling this out with the FF turned off, so that it operates correctly for everyone. I tested this branch, and there are no memory leaks with this FF turned off, which aligns with the thinking that the original need to have an experimental cleanup was due to our React usage which caused things to linger in memory without ability for the browser to ever garbage collect it.What libraries were added/updated?
None. Webpack config was the largest difference here, same version is used for all libs and plugins at this time.
Does this change affect performance?
It can increase performance due to the removal of many duplicated portions of code, but ultimately the performance gain initially here is in the more deterministic cache strategy around the code bundles shipped being more resistant to unnecessary change. This means less code being reloaded between versions shipped, and stronger cache lifetimes between releases which improves load times in any given page by shipping less code over time for all users.
Does this change affect security?
No.
What alternative approaches were there?
The one other alternative to the particular solution here was to fix the CSS prefixing collisions with a modified webpack config for the css_prefix value injected in css/scss/styl files during processing. This was attempted and blocked by the fact that in a given build, if the file which would be produced is a culmination of multiple other css from multiple potential sources (LS App, Editor, DM) it only could be made to apply a single css prefix holistically to the file output, and didn't allow for proper distinguishing between which files had sourced the particular css chunk. This ultimately meant that it was non trivial to achieve this level of change with Webpack, and decided to not pursue any further.
What feature flags were used to cover this change?
None, it wasn't possible to entertain many builds to possibly FF copies all over the place of similar files to retain the previous and the new. This would only further complicate the situation and make it even harder to move forward.
Does this PR introduce a breaking change?
(check only one)
Which logical domain(s) does this change affect?
All.